ENH: An initial implementation of SD estimation. #97
Conversation
|
Best reviewed: commit by commit
Optimal code review plan
|
Codecov Report
@@ Coverage Diff @@
## master #97 +/- ##
==========================================
- Coverage 51.37% 50.11% -1.26%
==========================================
Files 20 21 +1
Lines 1238 1289 +51
Branches 162 169 +7
==========================================
+ Hits 636 646 +10
- Misses 590 631 +41
Partials 12 12
Continue to review full report at Codecov.
|
|
Hello @oesteban, Thank you for updating! Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, Comment last updated at 2020-06-02 23:43:34 UTC |
oesteban
force-pushed
the
enh/init-sdc
branch
2 times, most recently
from
e8ec194
to
d390517
Compare
Just to make the diff of nipreps#97 a bit cleaner.
ENH: Minor refactor reorganizing base workflows, in prep for #97
| @@ -92,6 +92,9 @@ ENV FSLDIR="/usr/share/fsl/5.0" \ | |||
| AFNI_PLUGINPATH="/usr/lib/afni/plugins" | |||
| ENV PATH="/usr/lib/fsl/5.0:/usr/lib/afni/bin:$PATH" | |||
|
|
|||
| COPY .docker/fsl-6.0/bin/topup /usr/share/fsl/5.0/bin/topup | |||
There was a problem hiding this comment.
FYI-- if you want to bump up the fsl version (I've recently encountered a number of issues with the neurodebian fsl-5.0 libraries), I've worked out the following:
RUN apt-get update -qq \
&& apt-get install -y --no-install-recommends curl libquadmath0 \
&& curl -sSL https://fsl.fmrib.ox.ac.uk/fsldownloads/fsl-6.0.2-centos7_64.tar.gz | tar xz -C /usr/local \
--exclude='fsl/doc' \
--exclude='fsl/data/first' \
--exclude='fsl/data/atlases' \
--exclude='fsl/data/possum' \
--exclude='fsl/src' \
--exclude='fsl/extras/src' \
--exclude='fsl/bin/fslview*' \
--exclude='fsl/bin/FSLeyes' \
--exclude='fsl/bin/*_gpu*' \
--exclude='fsl/bin/*_cuda*' \
&& chmod 775 -R /usr/local/fsl/bin \
&& chown -R neuro /usr/local/fsl
It does add some GB to the build though...
There was a problem hiding this comment.
bumping fsl will definitely break nipype at points... so probably not the rabbit hole I'd like to explore right this minute... I agree in some time we will be updating to 6.0 (and then your code will be very useful).
There was a problem hiding this comment.
Yeah, probably best to wait then. I wish there was an organized bookmarking system on github so that we could more easily remember to revisit/ priority-mark these kinds of things!
Adds a new subworkflow based on FSL TOPUP to integrate SD estimation for the ds001771 dataset. - [x] Pin niworkflows to current master (while I release 1.2.0rc5 containing nipreps/niworkflows#503, nipreps/niworkflows#504, which are used here). - [x] Create a new sdc estimation workflow, with the expectation of upstreaming it to SDCFlows. - [x] Implement the barebones of how nipreps/sdcflows#101 could look like. Also to be upstreamed to SDCFlows when mature. - [x] Stick TOPUP from FSL 6.0.3 in the Docker image, since topup from FSL 5.0.x is really unstable (for instance, it fails with a segmentation fault on the workflow of ds001771) Resolves: nipreps#92
and fix THP002, and pin latest rc of niworkflows
Addressing the weak spot identified by @dPys (https://github.com/nipreps/dmriprep/pull/97/files#r416242563).
|
Alright, the report with TOPUP-estimated field is here - https://460-233429127-gh.circle-artifacts.com/0/tmp/ds001771/derivatives/dmriprep/sub-36.html |
Hi @oesteban -- this looks REALLY nice to me. Question-- do the reports need to visualize the inhomogeneity outside of the brain or do you think we should just mask it so that users don't get alarmed by the amount of out-of-brain field inhomogeneity? |
This question is THE QUESTION. On the one hand, I do agree it is alarming (especially to the non-familiar eye). On the other hand, I think it is nice to compare how different methods regularize outside the brain differently. For instance, the traditional FUGUE correction (the infamous Would you mind filing an issue about this at SDCflows? |
Adds a new subworkflow based on FSL TOPUP to integrate SD estimation
for the ds001771 dataset.
containing FIX: Merge/SplitSeries write to path of input image, instead of cwd niworkflows#503, FIX: Add DWI default patterns for dMRIPrep's reportlets niworkflows#504, which
are used here).
upstreaming it to SDCFlows.
like. Also to be upstreamed to SDCFlows when mature.
FSL 5.0.x is really unstable (for instance, it fails with a
segmentation fault on the workflow of ds001771)
Resolves: #92